SGP30 / SGP40 background sampling #811

mikeysklar · 2025-09-12T16:47:39Z

Add SGP30 / SGP40 background sampling. Requires frequently 1s polling or returns 0 values.

Untested and only a proof of concept for feedback. This adds fastTick() polling to keep the SGPxx sensors producing usable data. Thanks @Timeline for the key insight of 1s polling being needed.

Difference observed between 15min polling versus 1s with the SGP40.

Address issue 732

Forum issues:

SGP30 issue with WipperSnapper
SGP40 Air Quality

Add SGP30 / SGP40 background sampling. Requires frequently 1s polling or returns 0 values.

also added sgp30 averaging option

mikeysklar · 2025-09-14T17:05:57Z

I can confirm the SGP40 works with this PR. I tested using:

Adafruit Qt Py ESP32-S3 (2MB PSRAM)
build artifact: wippersnapper.qtpy_esp32s3_n4r2.1.0.0-adafruit-67159326.uf2

Trying different settings from 1 second --> 15 minute update intervals for prolonged periods (hours) was working with the keep alive code.

The SGP30 changes are untested as I don't have one on hand.

ladyada · 2025-09-15T11:25:53Z

brent and tyeth are both out right now but they'll review when return!

tyeth · 2025-09-15T11:38:25Z

Thanks @mikeysklar, this has been weighing on my mind for a while as there are a few we currently "support" that are out of spec in their current use. I thought there was an existing issue, but can't find one, must be thinking of forum discussions.

The only other previous related thoughts were that it might be worth having an internal poll period defined for each component (with a matching entry in the components repo JSON definition), and then a separate publish period (the current thing in definition file), both potentially displayed to the user (publish >= poll in interface) although we'd then need to specify max and min as well as default internal poll period (fastTick speed).
I think that would help users understand (and play out well in low power mode in the future) poll periods.
Also as you've seen, it wasn't worth reading a lot of sensors too frequently (like more than every second). I'm thinking some other air sensors which are 5seconds minimum poll for example. Longer term it's my desire (and been raised) to have the reference values (temp/RH) be passable to the gas sensors too as currently it (20-25c/50%/101.325) throws all the data off. That would either be input feeds for the sensor/component, or some form of custom sensor properties or RPC thing, probably through Actions initially.

On the averaging front, it might be best to leave it to the current techniques, as I think it's probably expected that the moment it publishes data the value represents the current value and not a time accumulated average. The user can see averaged data instead using the dashboard and feed pages and APIs (charts and API get access to aggregated data at various time intervals), and possibly Actions.
Longer term it makes sense to offer the sample averaging as a configurable option (similarly for Analog averaging and/or hysteresis).
There's also a minor issue with the getEvent situation, as each method resets the data, and usually they are called one after another, before a single publish, so first getEvent(VOC for sake of argument) takes the averaged value from _n samples, but then getEventRAW doesn't match that dataset as the accumulators are reset by getEventVOC and so instead getEventRaw asks for a fresh data sample from the sensor.

These are all minor things, and more an attempt to uncork my stored up thoughts on the issue. I'll bring it up with Brent as to how he'd like to proceed, which will be next week, but I can't see any problem with the current idea at all.

mikeysklar · 2025-09-15T13:06:46Z

@tyeth

Thank you for the larger explanation. Using a polling frequency value in the JSON config makes a lot of sense.

Another issue @Timeline pointed out is deep sleep mode which I believe is still in the works. I bring it up here only that continuous background polling could cancel out the ability to use sleep mode with these sensors.

A few more options to consider as JSON config values:

ramp up time (eg. 30sec - 1min)
number of initial values to throw away
deep sleep duration (realistic range)

If you are doing hourly air quality samples it doesn't make a lot of sense to keep the controller awake and polling every second.

brentru

Overall, this makes sense to me as an implementation for a few drivers in WipperSnapper beta.

We're currently implementing the new WipperSnapper API (v2) and have time to consider alternatives for specific things, like background sampling, into the new API design.

@tyeth If you also approve of this design, I wouldn't be opposed to merging it in. We can talk about it more tomorrow or in this PR.

@mikeysklar I've requested a few design/syntax changes. Please hold off on these until Tyeth also approves.

brentru · 2025-09-22T14:43:54Z

src/components/i2c/WipperSnapper_I2C.cpp

+  for (auto *drv : drivers) {
+    if (drv)
+      drv->fastTick();
+  }
+


Do not use auto here or create a loop. It is redundant and adds a performance overhead for what should be a tight loop.

Instead, try calling drv->fastTick() when we are looping through the vectors below. Since fastTick is virtual, and the vect. of drivers exists, you dont need to check for nullptr either.

brentru · 2025-09-22T14:44:06Z

src/components/i2c/WipperSnapper_I2C.cpp

  while (sensorsReturningFalse && retries > 0) {
    sensorsReturningFalse = false;
    retries--;
+    curTime = millis();


Why would curTime be modified here?

brentru · 2025-09-22T14:44:53Z

src/components/i2c/drivers/WipperSnapper_I2C_Driver_SGP40.h

+      return;
+
+    uint32_t now = millis();
+    if (now - _lastFastMs >= 1000) {


1000 is a magic number, please #define what it is at the top of this header.

brentru · 2025-09-22T14:45:10Z

src/components/i2c/drivers/WipperSnapper_I2C_Driver_SGP40.h

+  uint32_t _lastFastMs = 0;
+
+  /** Number of samples accumulated since last publish. */
+  uint32_t _n = 0;


_num_samples

brentru · 2025-09-22T14:47:46Z

src/components/i2c/drivers/WipperSnapper_I2C_Driver_SGP40.h

+    _n = 0;
+    _vocSum = 0.0f;
+    _rawSum = 0;


Move these to the constructor

src/components/i2c/drivers/WipperSnapper_I2C_Driver_SGP40.h

brentru · 2025-09-22T14:49:33Z

src/components/i2c/drivers/WipperSnapper_I2C_Driver_SGP30.h

+  uint32_t _lastFastMs = 0;
+
+  /** Number of samples accumulated since last publish. */
+  uint32_t _n = 0;


Please change to _sample_num or something more descriptive

src/components/i2c/drivers/WipperSnapper_I2C_Driver_SGP30.h

brentru · 2025-09-22T14:50:03Z

src/components/i2c/drivers/WipperSnapper_I2C_Driver_SGP30.h

+    if (!iaqEnabled())
+      return; // nothing enabled, save cycles
+    uint32_t now = millis();
+    if (now - _lastFastMs >= 1000) { // ~1 Hz cadence


1000 is a magic number, please #define it.

mikeysklar · 2025-09-22T15:11:13Z

@brentru -

Welcome back. Thank you for the thorough review.

I'll hold off on the improved variable names and getting magic number defined updates until you and @tyeth discuss this.

tyeth · 2025-09-25T13:25:29Z

Another issue @Timeline pointed out is deep sleep mode which I believe is

still in the works. I bring it up here only that continuous background polling could cancel out the ability to use sleep mode with these sensors. That was my nod to Low Power mode, a.k.a. deep sleep support, which will be v2 of WipperSnapper (both Offline and Online mode - currently only Offline mode <https://github.com/adafruit/Adafruit_Wippersnapper_Arduino/tree/offline-mode/src> uses v2 codebase).

A few more options to consider as JSON config values: * ramp up time (eg. 30sec - 1min) * number of initial values to throw away * deep sleep duration (realistic range)

Yeah I like these. Less so the deep sleep duration, or at least it then prompted a bit of confusion. I initially imagined deep-sleep-duration as unnecessary because it's just the normal polling period people select, but then realise maybe you're referring to the usable range for low power / sleep usage which gets more complicated/useful. I wonder if ramp up time is the time until good data, or just any semi-usable data, and whether users would prefer to still report it (advanced config option?) during that period. Or is ramp up simply until any data that shouldnt be 0/NaN/etc, like 15s for SGP30 mentioned later, not until the internal algorithm is happy (do we need another field related to stability/quality of data after x amount of time).

If you are doing hourly air quality samples it doesn't make a lot of

sense to keep the controller awake and polling every second. Agreed, the one issue in the back of my mind is the air quality sensors (or any type) that have an inbuilt algorithm, rather than raw data (Sensirion VOC/IAQ index plus some others). They require the baseline values/coefficients to be trained for ages and regularly stored (ideally after each reading) and even then are [only valid for the device being asleep about 10mins max]( https://github.com/adafruit/Adafruit_SGP40/blob/741a139e73e95273d6b5ac074c780aa74597bb9e/src/sensirion_voc_algorithm.h#L133C51-L134C52) (otherwise [the full baseline compensation run must be performed at next wake/boot]( https://github.com/adafruit/Adafruit_SGP40/blob/741a139e73e95273d6b5ac074c780aa74597bb9e/src/sensirion_voc_algorithm.h#L122-L123) ). I guess that 10minute limit would be the "max deep sleep duration", assuming the "ramp up time" was 3hours and not performed on wake (so not after a <10min low power sleep), and we probably want to do the fastest tick possible when throwing away values (rather than waiting for poll period before broadcasting first value on a fresh boot - maybe not for a waking boot). In the back of my mind I'm thinking that throw-away-values might fall apart quickly if the sensor has a slow internal poll period (calling that fastTicks in this PR), like the 5seconds minimum poll period on one air one (I forget which - SCD?). I guess we'd just drop the first value and no others in that situation (it's real CO2 not algorithm based so 30s ramp up IIRC). I'm not sure in the air sensor situation what a user would prefer/expect, personally I want correct data more than power saving, so ideally I'd see the first boot not put the sensor to sleep until at least 3hours had elapsed so the baseline could be saved. I guess it could sleep the mcu almost immediately as long as the sensor remains powered, and it internally keeps updating the algorithm without being read (assuming in appropriate mode) like the SEN4x/5x do (internally updating the algo at 1Hz IIRC). Forgetting those Sensirion ones for a second, and still looking at VOC, then we only sell the ENS160 (<3 minutes warm-up <1 hour start-up), and the BME68x (30mins each startup). Switching to look at CO2, or better yet eCO2 (estimated/equivalent), then we only really have the ENS160 + Sensirion SGP and SCD series. Taking the SGP30 we could benefit from dropping the first 15seconds of data <https://cdn-learn.adafruit.com/assets/assets/000/050/058/original/Sensirion_Gas_Sensors_SGP30_Datasheet_EN.pdf#:~:text=first%2015s> (ramp up time?) before the algo wakes up, use an internal poll of 1second (which the algo prefers <https://cdn-learn.adafruit.com/assets/assets/000/050/058/original/Sensirion_Gas_Sensors_SGP30_Datasheet_EN.pdf#:~:text=The%20on>, but doesn't work well for low power). It suggests the baseline is useful for up to a week <https://files.seeedstudio.com/wiki/Grove-VOC_and_eCO2_Gas_Sensor-SGP30/res/Sensirion_Gas_Sensors_SGP30_Driver-Integration-Guide_HW_I2C.pdf#:~:text=one%20week>, and initially only valid after 12hours <https://files.seeedstudio.com/wiki/Grove-VOC_and_eCO2_Gas_Sensor-SGP30/res/Sensirion_Gas_Sensors_SGP30_Driver-Integration-Guide_HW_I2C.pdf#:~:text=12%20hours>. So how would one expect that to play out?

…

On Mon, 22 Sept 2025 at 16:11, Mikey Sklar ***@***.***> wrote: *mikeysklar* left a comment (adafruit/Adafruit_Wippersnapper_Arduino#811) <#811 (comment)> @brentru <https://github.com/brentru> - Welcome back. Thank you for the thorough review. I'll hold off on the improved variable names and getting magic number defined updates until you and @tyeth <https://github.com/tyeth> discuss this. — Reply to this email directly, view it on GitHub <#811 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABTBZ4ZTXXSFKBNRLT5UIFT3UAGSRAVCNFSM6AAAAACGLRAZG2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTGMJZGY2DOMJVGI> . You are receiving this because you were mentioned.Message ID: ***@***.***>

tyeth · 2025-09-25T14:11:07Z

src/components/i2c/drivers/WipperSnapper_I2C_Driver.h

  /*******************************************************************************/
  /*!
-      @brief    Instanciates an I2C sensor.
+      @brief    Instanctiates an I2C sensor.


no middle c

brentru

@mikeysklar Tyeth and I discussed this fix this morning. You are on the right track, but we'd like to remove the averaging code.

For the SGP30,

Remove all the code related to averaging values within the driver.
fastTick is OK but should only just read the driver. It should set a boolean flag if IAQmeasure succeeded.
getEventX() methods should first check the validity of the previous read (return value of IAQmeasure()) from fastTick, and fill the Event if the read was OK.

For the SGP40,

Remove all the code related to averaging values within the driver.
Add uint16_t _rawValue and int32_t _vocIdx as protected: members
fastTick should just poll for raw value and voc inx, and save those into _rawValue and _vocIdx.
Within getEventX(), pack _rawValue or _vocIdx into an Event.

I added two comments below to reflect these changes. lmk if you need more info/have more questions, and thanks for taking a look at this. If you'd like Tyeth to take this over as well, let him know.

src/components/i2c/drivers/WipperSnapper_I2C_Driver_SGP30.h

mikeysklar · 2025-09-25T19:52:46Z

@brentru - Thank you for taking the time to make the above suggested changes. I'll start updating the PR with what you have requested and confirm functionality before asking for another review.

@tyeth - The mixed cohorts of the various sensors do complicate things. My feeling is that if a sensors needs 30 minutes to get its bearings then it should be blocked from sleep. I'm looking at your BME68x. Maybe it makes sense to build on the per sensor JSON configs that WS v2 introduces and have fairly elaborate JSON config knobs.

eg.

{
  "sensor_id": "sgp30-1",
  "class": "A",
  "ramp_up_s": 20,
  "min_poll_s": 1,
  "throwaway_n": 3,
  "commission_time_s": 43200,        // e.g., 12h SGP30 baseline build
  "persist_baseline": true,
  "max_sleep_s": 600,                // 10 min guard for algo continuity
  "keep_powered_in_sleep": true,     // overrides if sample period > max_sleep_s
  "publish_during_ramp": false,      // advanced users can set true
  "quality_threshold": {
    "variance_ppm": 50,              // stop throwing away once stabilized
    "min_samples": 3
  }
}

then rather than fighting for quality data publish the state of the data:

{
  "sensor_id": "sgp30-1",
  "state": "stable|commissioning|warming_up|resume_after_sleep|rebaseline",
  "ramp_elapsed_s": 28,
  "baseline_age_h": 3.2,
  "samples_discarded": 3
}

…emove averaging, cache single reads; minor doc/guard fixes

mikeysklar · 2025-09-26T14:38:02Z

Current state of PR. Testing after averaging / corrections made went fine overnight for 1s, 30s, 1m, 10m intervals checks. I messed with air quality levels and the sensor was very responsive. The only concern I have in terms of functionality is I've pulled the 1Hz timer out of fastTick(). I should probably be put back so the sensor is not getting queried at main loop speed.

brentru · 2025-09-26T15:49:43Z

@mikeysklar Sorry, what 1Hz timer?

mikeysklar · 2025-09-26T16:16:05Z

I had a little millis counter with the 1000 magic number in the SGP30 / SGP40 code to poll the sensors at 1Hz. I ripped it up yesterday, but should probably put that back in to prevent constant polling.

    if (now - _lastFastMs >= 1000) { // ~1 Hz cadence

brentru · 2025-09-26T18:56:06Z

Oh, yeah, that should go back in

mikeysklar · 2025-09-28T00:11:15Z

I've been testing the SGP40 and it has been running smoothly with different update periods 1s - 10min. I believe I got all the suggested changes in. I ordered a SGP30 for testing which I can confirm functionality this week.

brentru

@mikeysklar I only have one change to request. Looks great.

@tyeth I've tagged you for secondary review since this touches the WipperSnapper_I2C class.

src/components/i2c/drivers/WipperSnapper_I2C_Driver_SGP30.h

mikeysklar · 2025-09-30T17:26:14Z

thanks @tyeth -

I like simple. I'm going to change up the SGP30 to look more like the SGP40 and just use the 1 Hz fastTick for sensor polling. No more consumed cache flag which is confusing.

mikeysklar · 2025-09-30T20:02:09Z

SGP30 arrived today so I'll try running both SGP30 and SGP40 with the latest artifacts and report back.

mikeysklar · 2025-10-01T14:36:23Z

I ran the SGP30 / SGP40 / SCD41 overnight at 5 minute intervals. Data was reliably coming and correlates to some degree. Keep in mind the SGP30 is measuring VOC in (PPB) while the SGP40 has its own index value of 0-500.

Keep in mind that the SGP30 is a new sensor fresh out of the bag so has not been through a 48 hour burn-in period yet.

brentru

@mikeysklar LGTM! Thank you for adding this PR

@tyeth Please pull this in when you are ready.

tyeth · 2025-10-01T18:32:22Z

@mikeysklar released as v1.0.0-beta.117

mikeysklar · 2025-10-01T19:06:42Z

Cool! Thank you @tyeth and @brentru for your patience. Glad to see fastTicks() is already being extended to additional devices.

mikeysklar added 4 commits September 12, 2025 09:42

SGP30 / SGP40 background sampling

c9ec675

Add SGP30 / SGP40 background sampling. Requires frequently 1s polling or returns 0 values.

removed override / const

0b020d4

clang

f98d42b

clang whitespace

662f61c

mikeysklar force-pushed the sgpxx-faster branch 3 times, most recently from 0ee75ca to 662f61c Compare September 13, 2025 02:35

mikeysklar added 2 commits September 13, 2025 08:02

sgp30 doxygen

b1154e2

sgp40 docs doxygen

5cbb3d5

ladyada requested review from brentru and tyeth September 13, 2025 17:04

mikeysklar added 2 commits September 14, 2025 08:17

sgp30 / sgp40 integer values

281e41d

also added sgp30 averaging option

style: apply clang-format to SGP30 driver

222e45b

brentru requested changes Sep 22, 2025

View reviewed changes

tyeth reviewed Sep 25, 2025

View reviewed changes

brentru requested changes Sep 25, 2025

View reviewed changes

src/components/i2c/drivers/WipperSnapper_I2C_Driver_SGP30.h Show resolved Hide resolved

src/components/i2c/drivers/WipperSnapper_I2C_Driver_SGP30.h Show resolved Hide resolved

mikeysklar added 4 commits September 25, 2025 16:44

I2C: per-driver fastTick call site; fix fastTick docs; SGP30/SGP40: r…

1dc39ca

…emove averaging, cache single reads; minor doc/guard fixes

clang format

fe0ca83

docs: add Doxygen comment for _vocIdx in SGP40 driver

f9dbb5e

docs: concise VOC index comment

557a0f4

mikeysklar added 2 commits September 26, 2025 17:39

SGP30/40: add fastTick 1Hz cadence, simplify caching, update docs

0a0927d

clang whitespace

7e6e75c

brentru requested review from brentru and tyeth September 29, 2025 13:55

brentru requested changes Sep 29, 2025

View reviewed changes

src/components/i2c/drivers/WipperSnapper_I2C_Driver_SGP30.h Outdated Show resolved Hide resolved

remove override on sgp30/sgp40 destructors

7f7f4dd

tyeth requested changes Sep 30, 2025

View reviewed changes

src/components/i2c/drivers/WipperSnapper_I2C_Driver_SGP30.h Outdated Show resolved Hide resolved

mikeysklar added 2 commits September 30, 2025 11:00

sgp30 fastTick only, remove consume cache confusion

ed2fe67

SGP30: align Doxygen / clang

a0fdee7

tyeth approved these changes Oct 1, 2025

View reviewed changes

brentru approved these changes Oct 1, 2025

View reviewed changes

tyeth merged commit a94544f into adafruit:main Oct 1, 2025
46 checks passed

tyeth mentioned this pull request Oct 1, 2025

Refactor other sensors to use fastTicks / internal poll period #822

Open

SGP30 / SGP40 background sampling #811

SGP30 / SGP40 background sampling #811

Uh oh!

Conversation

mikeysklar commented Sep 12, 2025

Uh oh!

mikeysklar commented Sep 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ladyada commented Sep 15, 2025

Uh oh!

tyeth commented Sep 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mikeysklar commented Sep 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

brentru left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mikeysklar commented Sep 22, 2025

Uh oh!

tyeth commented Sep 25, 2025 via email

Uh oh!

Choose a reason for hiding this comment

Uh oh!

brentru left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mikeysklar commented Sep 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mikeysklar commented Sep 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

brentru commented Sep 26, 2025

Uh oh!

mikeysklar commented Sep 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

brentru commented Sep 26, 2025

Uh oh!

mikeysklar commented Sep 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

brentru left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mikeysklar commented Sep 30, 2025

Uh oh!

mikeysklar commented Sep 30, 2025

Uh oh!

mikeysklar commented Oct 1, 2025

Uh oh!

brentru left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tyeth commented Oct 1, 2025

Uh oh!

mikeysklar commented Sep 14, 2025 •

edited

Loading

tyeth commented Sep 15, 2025 •

edited

Loading

mikeysklar commented Sep 15, 2025 •

edited

Loading

mikeysklar commented Sep 25, 2025 •

edited

Loading

mikeysklar commented Sep 26, 2025 •

edited

Loading

mikeysklar commented Sep 26, 2025 •

edited

Loading

mikeysklar commented Sep 28, 2025 •

edited

Loading